Skip to content

Conversation

@brawner
Copy link
Contributor

@brawner brawner commented Aug 28, 2020

Found this memory leak during fault injection tests. It's pretty small, but it prevents the unit tests from being memory leak free.

Signed-off-by: Stephen Brawner [email protected]

Copy link
Contributor

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but what happened with the PR job of rcl? was it deactivated?

@clalancette
Copy link
Contributor

LGTM, but what happened with the PR job of rcl? was it deactivated?

No, it didn't trigger because this isn't targeting master. @brawner should we just target master with this PR, and then you can rebase your other work on top?

@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020

@clalancette I think that's reasonable. The refactor PR (#754) is going to make it much harder to backport fixes like this and #765.

@brawner brawner force-pushed the brawner/rcl_yaml-set-values-null branch from 4a633e2 to f8b7a70 Compare August 31, 2020 17:39
@brawner brawner force-pushed the brawner/rcl_yaml-add_val_to_string-leak branch from a9bab89 to 7cd635f Compare August 31, 2020 17:50
@brawner brawner changed the base branch from brawner/rcl_yaml-set-values-null to master August 31, 2020 17:51
@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020

Rebased onto master, testing --packages-select rcl_yaml_param_parser

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020

Rpr job still fails waiting for rcutils debian packages to be released. The ci.ros2.org jobs all pass.

@brawner brawner merged commit d716ad1 into master Aug 31, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/rcl_yaml-add_val_to_string-leak branch August 31, 2020 18:19
brawner added a commit that referenced this pull request Oct 5, 2020
@brawner
Copy link
Contributor Author

brawner commented Oct 5, 2020

Choosing not to backport this one, since the change is superseded anyway by #784

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants